Remove unnecessary shouldRunNow global variable. NFC
#23191
Conversation
| {{{ makeModuleReceiveWithVar('noInitialRun', undefined, !INVOKE_RUN) }}} | ||
| #if MAIN_READS_PARAMS | ||
| if (shouldRunNow) callMain(args); | ||
| if (!noInitialRun) callMain(args); |
There was a problem hiding this comment.
It would be helpful if we could check for Module['noInitialRun'] here rather than copying it into a variable. I have a use case with memory snapshots where if we have a memory snapshot we want to not run main. But I want to get the other Emscripten setup logic going while waiting on the snapshot network requests. So I use addRunDependency and then when the request comes back if I get a snapshot I set Module.noInitialRun to true and do other setup. I need to patch this line for this to work.
There was a problem hiding this comment.
Ah but I guess this change already fixes this problem for me because this new code assigns noInitialRun directly before reading it.
There was a problem hiding this comment.
It would be helpful if we could check for
Module['noInitialRun']here rather than copying it into a variable. I have a use case with memory snapshots where if we have a memory snapshot we want to not run main. But I want to get the other Emscripten setup logic going while waiting on the snapshot network requests. So I useaddRunDependencyand then when the request comes back if I get a snapshot I setModule.noInitialRuntotrueand do other setup. I need to patch this line for this to work.
While I'm glad that works, that doesn't sound like something that emscripten officially supporteds. I'm glad this changes makes it better, but I'm afraid we don't make any promises that will continue to work in the future.
There was a problem hiding this comment.
Is there a reason not to allow setting Module.noInitialRun in preRun hooks? It seems to me that as long as it is set ahead of callMain() it ought to work. At least as long as all it does is suppress this one call.
There was a problem hiding this comment.
Anyways we use a lot of emscripten implementation details and certainly don't expect the upgrade process to be simple. The most common problems are weird llvm regressions in compilation of C++ code.
There was a problem hiding this comment.
Yes, that seems pretty reasonable.
Our policy on incoming Module settings (i.e. things in INCOMING_MODULE_JS_API) is generally that they are read once on import/load and not again.
So this change actually going somewhat against this policy since its reading this value from the Module each time run is called. But that sounds like its actually going to help your use case, right?
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (63) test expectation files were updated by running the tests with `--rebaseline`: browser/test_small_js_flags.js.size: 4345 => 4327 [-18 bytes / -0.41%] other/codesize/test_codesize_cxx_ctors1.gzsize: 8542 => 8532 [-10 bytes / -0.12%] other/codesize/test_codesize_cxx_ctors1.jssize: 20925 => 20904 [-21 bytes / -0.10%] other/codesize/test_codesize_cxx_ctors2.gzsize: 8526 => 8516 [-10 bytes / -0.12%] other/codesize/test_codesize_cxx_ctors2.jssize: 20893 => 20872 [-21 bytes / -0.10%] other/codesize/test_codesize_cxx_except.gzsize: 9572 => 9564 [-8 bytes / -0.08%] other/codesize/test_codesize_cxx_except.jssize: 24770 => 24749 [-21 bytes / -0.08%] other/codesize/test_codesize_cxx_except_wasm.gzsize: 8503 => 8491 [-12 bytes / -0.14%] other/codesize/test_codesize_cxx_except_wasm.jssize: 20819 => 20797 [-22 bytes / -0.11%] other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize: 8503 => 8491 [-12 bytes / -0.14%] other/codesize/test_codesize_cxx_except_wasm_exnref.jssize: 20819 => 20797 [-22 bytes / -0.11%] other/codesize/test_codesize_cxx_lto.gzsize: 8440 => 8432 [-8 bytes / -0.09%] other/codesize/test_codesize_cxx_lto.jssize: 20504 => 20483 [-21 bytes / -0.10%] other/codesize/test_codesize_cxx_mangle.gzsize: 9573 => 9566 [-7 bytes / -0.07%] other/codesize/test_codesize_cxx_mangle.jssize: 24770 => 24749 [-21 bytes / -0.08%] other/codesize/test_codesize_cxx_noexcept.gzsize: 8542 => 8532 [-10 bytes / -0.12%] other/codesize/test_codesize_cxx_noexcept.jssize: 20925 => 20904 [-21 bytes / -0.10%] other/codesize/test_codesize_cxx_wasmfs.gzsize: 3802 => 3792 [-10 bytes / -0.26%] other/codesize/test_codesize_cxx_wasmfs.jssize: 8562 => 8541 [-21 bytes / -0.25%] other/codesize/test_codesize_files_js_fs.gzsize: 7679 => 7666 [-13 bytes / -0.17%] other/codesize/test_codesize_files_js_fs.jssize: 18832 => 18811 [-21 bytes / -0.11%] other/codesize/test_codesize_files_wasmfs.gzsize: 2897 => 2887 [-10 bytes / -0.35%] other/codesize/test_codesize_files_wasmfs.jssize: 6203 => 6183 [-20 bytes / -0.32%] other/codesize/test_codesize_hello_O0.gzsize: 8017 => 8020 [+3 bytes / +0.04%] other/codesize/test_codesize_hello_O0.jssize: 21521 => 21550 [+29 bytes / +0.13%] other/codesize/test_codesize_hello_O1.gzsize: 2753 => 2738 [-15 bytes / -0.54%] other/codesize/test_codesize_hello_O1.jssize: 6998 => 6970 [-28 bytes / -0.40%] other/codesize/test_codesize_hello_O2.gzsize: 2402 => 2388 [-14 bytes / -0.58%] other/codesize/test_codesize_hello_O2.jssize: 4911 => 4891 [-20 bytes / -0.41%] other/codesize/test_codesize_hello_O3.gzsize: 2312 => 2307 [-5 bytes / -0.22%] other/codesize/test_codesize_hello_O3.jssize: 4758 => 4738 [-20 bytes / -0.42%] other/codesize/test_codesize_hello_Os.gzsize: 2312 => 2307 [-5 bytes / -0.22%] other/codesize/test_codesize_hello_Os.jssize: 4758 => 4738 [-20 bytes / -0.42%] other/codesize/test_codesize_hello_Oz.gzsize: 2295 => 2288 [-7 bytes / -0.31%] other/codesize/test_codesize_hello_Oz.jssize: 4725 => 4705 [-20 bytes / -0.42%] other/codesize/test_codesize_hello_dylink.gzsize: 6191 => 6180 [-11 bytes / -0.18%] other/codesize/test_codesize_hello_dylink.jssize: 13684 => 13662 [-22 bytes / -0.16%] other/codesize/test_codesize_hello_wasmfs.gzsize: 2312 => 2307 [-5 bytes / -0.22%] other/codesize/test_codesize_hello_wasmfs.jssize: 4758 => 4738 [-20 bytes / -0.42%] other/codesize/test_codesize_libcxxabi_message_O3.gzsize: 1889 => 1882 [-7 bytes / -0.37%] other/codesize/test_codesize_libcxxabi_message_O3.jssize: 3995 => 3977 [-18 bytes / -0.45%] other/codesize/test_codesize_libcxxabi_message_O3_standalone.gzsize: 1928 => 1920 [-8 bytes / -0.41%] other/codesize/test_codesize_libcxxabi_message_O3_standalone.jssize: 4043 => 4025 [-18 bytes / -0.45%] other/codesize/test_codesize_mem_O3.gzsize: 2349 => 2336 [-13 bytes / -0.55%] other/codesize/test_codesize_mem_O3.jssize: 4899 => 4878 [-21 bytes / -0.43%] other/codesize/test_codesize_mem_O3_grow.gzsize: 2497 => 2486 [-11 bytes / -0.44%] other/codesize/test_codesize_mem_O3_grow.jssize: 5184 => 5163 [-21 bytes / -0.41%] other/codesize/test_codesize_mem_O3_grow_standalone.gzsize: 2192 => 2183 [-9 bytes / -0.41%] other/codesize/test_codesize_mem_O3_grow_standalone.jssize: 4592 => 4571 [-21 bytes / -0.46%] other/codesize/test_codesize_mem_O3_standalone.gzsize: 2158 => 2148 [-10 bytes / -0.46%] other/codesize/test_codesize_mem_O3_standalone.jssize: 4522 => 4501 [-21 bytes / -0.46%] other/codesize/test_codesize_mem_O3_standalone_lib.gzsize: 1914 => 1907 [-7 bytes / -0.37%] other/codesize/test_codesize_mem_O3_standalone_lib.jssize: 4042 => 4024 [-18 bytes / -0.45%] other/codesize/test_codesize_mem_O3_standalone_narg.gzsize: 1928 => 1920 [-8 bytes / -0.41%] other/codesize/test_codesize_mem_O3_standalone_narg.jssize: 4043 => 4025 [-18 bytes / -0.45%] other/codesize/test_codesize_mem_O3_standalone_narg_flto.gzsize: 1928 => 1920 [-8 bytes / -0.41%] other/codesize/test_codesize_mem_O3_standalone_narg_flto.jssize: 4043 => 4025 [-18 bytes / -0.45%] other/codesize/test_codesize_minimal_pthreads.gzsize: 4155 => 4146 [-9 bytes / -0.22%] other/codesize/test_codesize_minimal_pthreads.jssize: 8612 => 8594 [-18 bytes / -0.21%] other/test_INCOMING_MODULE_JS_API.js.size: 3758 => 3740 [-18 bytes / -0.48%] other/test_unoptimized_code_size.js.size: 53979 => 53946 [-33 bytes / -0.06%] other/test_unoptimized_code_size_no_asserts.js.size: 29399 => 29317 [-82 bytes / -0.28%] other/test_unoptimized_code_size_strict.js.size: 52775 => 52768 [-7 bytes / -0.01%] Average change: -0.28% (-0.58% - +0.13%)
3 participants
No description provided.